Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build,refactor: update development tooling and package configuration #614

Merged
merged 85 commits into from
Dec 20, 2024

Conversation

egparedes
Copy link
Contributor

@egparedes egparedes commented Nov 29, 2024

This PR moves the development tooling to use modern and more capable tools like uv and nox. It also adds a new icon4py virtual package in the root folder (used to gather all the model components) and moves out of the icon4py.model.common subpackage to the new icon4py.model.testing subpackage the utilities only needed for testing purposes.

uv tool is used to manage the dependencies of all subprojects thanks to its support for the workspace concept which fits perfectly the monorepo structure of this repository. All requirements files have been replaced by dependency groups in the root pyproject.toml used by uv.

nox is used to manage the testing environments instead of tox. Feature-wise is similar to tox but it has a more user-friendly configuration using Python code instead of configuration files.

@@ -82,4 +82,4 @@ RUN pyenv update && \
ENV PATH="/root/.pyenv/shims:${PATH}"

ARG CUPY_PACKAGE=cupy-cuda11x
RUN pip install --upgrade pip setuptools wheel tox clang-format ${CUPY_PACKAGE}
RUN pip install --upgrade pip setuptools wheel uv nox clang-format ${CUPY_PACKAGE}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RUN pip install --upgrade pip setuptools wheel uv nox clang-format ${CUPY_PACKAGE}
RUN pip install --upgrade uv nox

Probably that's the only thing that is useful here, because later we create a venv, which will not contain the other stuff.
Therefore, we don't have cupy in the environment, therefore the gpu test in tests/py2fgen/test_cli.py fails. Not sure what's the best way to define the cupy package version and then install it with nox/uv.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more cleanup can be applied to the Dockerfile, but as discussed with @egparedes it can be done after this PR is merged.

For example, the pyenv is probably not needed and these lines can be removed:

ENV PYENV_ROOT /root/.pyenv
ENV PATH="/root/.pyenv/bin:${PATH}"

RUN pyenv update && \
    pyenv install ${PYVERSION} && \
    echo 'eval "$(pyenv init -)"' >> /root/.bashrc && \
    eval "$(pyenv init -)" && \
    pyenv global ${PYVERSION}

ENV PATH="/root/.pyenv/shims:${PATH}"

Instead, we could just install uv with:

RUN curl -LsSf https://astral.sh/uv/install.sh | sh
ENV PATH="/root/.local/bin:${PATH}"

@havogt
Copy link
Contributor

havogt commented Dec 19, 2024

cscs-ci run default

noxfile.py Outdated
@nox.session(python=["3.10", "3.11"])
@nox.parametrize("subpackage", MODEL_SUBPACKAGE_PATHS)
def test_model_stencils(session: nox.Session, subpackage: ModelSubpackagePath) -> None:
if subpackage != "common": # TODO: Enable tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the stencil_tests from common were not run in main. Not sure if on purpose or by mistake.

@havogt
Copy link
Contributor

havogt commented Dec 19, 2024

cscs-ci run default

ci/base.yml Outdated
@@ -81,6 +81,7 @@ build_image_aarch64:
variables:
CSCS_ADDITIONAL_MOUNTS: '["/project/d121/icon4py/ci/testdata:$TEST_DATA_PATH"]'
HPC_SDK_PATH: "/opt/nvidia/hpc_sdk/Linux_${ARCH}/22.11"
ICON4PY_NOX_UV_EXTRA_ARGS: "--extra=cuda11"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, the cupy-cuda python package installed in the system environment in Docker build is not available in the uv environment. We can remove it from the Docker file, I guess.

noxfile.py Outdated Show resolved Hide resolved
noxfile.py Outdated Show resolved Hide resolved
@edopao
Copy link
Contributor

edopao commented Dec 20, 2024

cscs-ci run default

@edopao
Copy link
Contributor

edopao commented Dec 20, 2024

cscs-ci run dace

Comment on lines +85 to +86
elif subpackage == "common":
session.skip(f"tests broken") # TODO: Enable tests
Copy link
Contributor

@havogt havogt Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ci/dace.yml Outdated Show resolved Hide resolved
@egparedes
Copy link
Contributor Author

cscs-ci run default

Copy link

Mandatory Tests

Please make sure you run these tests via comment before you merge!

  • cscs-ci run default
  • launch jenkins spack

Optional Tests

To run benchmarks you can use:

  • cscs-ci run benchmark

To run tests and benchmarks with the DaCe backend you can use:

  • cscs-ci run dace

In case your change might affect downstream icon-exclaim, please consider running

  • launch jenkins icon

For more detailed information please look at CI in the EXCLAIM universe.

@egparedes
Copy link
Contributor Author

cscs-ci run default

@egparedes
Copy link
Contributor Author

cscs-ci run dace

@egparedes egparedes merged commit 353b210 into main Dec 20, 2024
3 of 4 checks passed
@edopao edopao mentioned this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants